Fake mod containers #192

LemmaEOF · 2020-01-06T22:51:37Z

Mainly intended as a conversation starter, absolutely not merge-ready

Adds a system for adding "fake" mods which exist primarily as metadata storage and do not have any entrypoints called or mixins applied. Intended for systems which want to add "mods" as metadta for systems like Mod Menu to access. Possibly not helpful in the first place.

Currently runs the fake_mods entrypoint right after main is run. No system is yet provided for removing fake mods or calling after mods are loaded.

i509VCB

Just a few comments with a few questions.

One addition I would like to see is a way to get a list of fake/provided mods, possibly a:

Collection<ProvidedModContainer> getModsProvidedFrom(ModContainer);

One contract of that above method would be that, the ModContainer used to get the providers MUST be a real mod.

I think we should add a isFake method to ModContainer.

For an extended ModContainer type for the fake containers I think we should have a ProvidedModContainer where there is a getProvider method so we can figure out which mod provided the fake container.

i509VCB · 2020-01-06T23:33:01Z

src/main/java/net/fabricmc/loader/entrypoint/minecraft/hooks/EntrypointClient.java

@@ -30,6 +31,7 @@ public static void start(File runDir, Object gameInstance) {

 		FabricLoader.INSTANCE.prepareModInit(runDir, gameInstance);
 		EntrypointUtils.invoke("main", ModInitializer.class, ModInitializer::onInitialize);
+		FabricLoader.INSTANCE.getEntrypoints("fake_mods", FakeModProvider.class).forEach(provider -> FabricLoader.INSTANCE.appendFakeMods(provider.getFakeMods()));


The question I have is, is this the right spot for the FakeModProviders to run?

I personally think that we should load fake mods after the GameProvider is set but before onInitialize after each of the ModInitializers fires their constructors.
One key reason for doing this before Client/Common/Server entrypoints is a concern about mod loading order, where if you do this after the Common Initializer has gone off you don't exactly have much context in what all mods that exist at that point along with breaking the expectation of the ModInitalizers having a final state for mods being loaded.

The main worry there is that then the fake mods likely haven't been loaded. You'd almost definitely want to do that in onInitialize.

What is your reason for loading fake mods after the mod initializer (thereby the client/server would be loaded). Sponge's events cover some states near what on Initialize would cover.

i509VCB · 2020-01-06T23:39:13Z

src/main/java/net/fabricmc/api/FakeModProvider.java

+ * Should run <i>after</i> {@link ModInitializer#onInitialize()}
+ */
+public interface FakeModProvider {
+	Collection<ModContainer> getFakeMods();


Should this be like below instead?

void registerFakeMods(Consumer<ProvidedModContainer> acceptor);

That would allow for a few things:
No need to worry about the whole collection being passed as null.
Anyone using the method would do the following for example:

public void registerFakeMods(Consumer<ProvidedModContainer> acceptor) { ProvidedModContainer container1 = ... ProvidedModContainer container2 = ... acceptor.accept(container1); acceptor.accept(container2); }

that might be a good idea. I'll think about it.

i509VCB · 2020-01-06T23:40:20Z

src/main/java/net/fabricmc/loader/FabricLoader.java

 	@Override
 	public boolean isModLoaded(String id) {
-		return modMap.containsKey(id);
+		return modMap.containsKey(id) || fakeModMap.containsKey(id);


My request for this would require a contract to be established that a IllegalArgumentException would be thrown if two ModContainers of the same modid were to be registered during the registration process.

oh that's a good point.

i509VCB · 2020-01-06T23:51:19Z

src/main/java/net/fabricmc/api/FakeModProvider.java

@@ -0,0 +1,13 @@
+package net.fabricmc.api;
+
+import net.fabricmc.loader.api.ModContainer;


There is currently not way to distinguish between a normal and fake mod container.
First I think we should add a isFake/isProvided method.

I think we should have return a ProvidedModContainer which extends ModContainer and a ModContainer getProvider(); so we could track which mod made the fake mod. Of course this would require a way to get what ModContainer provides an entrypoint (so we have a source of the FakeModProvider).

i509VCB · 2020-01-06T23:54:28Z

src/main/java/net/fabricmc/loader/api/FabricLoader.java

@@ -59,6 +59,12 @@ static FabricLoader getInstance() {
 	 */
 	Collection<ModContainer> getAllMods();


Should getAllMods() return every single mod, regardless of weather it's fake or real and add a getRealMods() method.

Or should we make this exclusively real mods.

I feel like it'd be better to have near-total separation between fake and real mods. I might even want to have isModLoaded have a counterpart isFakeModLoaded instead of the || behavior it has in the current state of the PR.

I disagree with boundary here, I think mods should be all equal by default. If someone provides a "fake" mod with the same id as a real mod, there should be a good reason, and they should be treated the same.

modmuss50 · 2020-01-07T01:35:19Z

I’m not sure fake is the right word here. As it could well be something that resembles a real mod (to the user anyway).

We use builtin for the game. Provided mod? Proxy mod? I’m not really too sure.

i509VCB · 2020-01-07T01:51:06Z

We use builtin for the game. Provided mod? Proxy mod? I’m not really too sure.

I've been referring to these ProvidedModContainers. Proxy mod makes it sound like a real mod hides behind it.

Pyrofab · 2020-01-07T08:04:20Z

I think we shouldn't limit this API to fake mods. There are several use cases in which provided mods should have full capabilities, actual fake mods are merely a special case. In fact, couldn't fake mods simply be regular mods with no source ?

LemmaEOF · 2020-01-07T08:05:52Z

Yeah, that is a good point. Maybe it'd be better for the provider to just let mods add new candidate finders to the ModResolver.

LemmaEOF · 2020-01-07T08:08:11Z

The only issue there is, of course, load problems. You shouldn't exactly have mods be able to add mod resolvers at a time when mods aren't even loaded yet. It seems like a big nightmare mess. Maybe we could use Java-native ServiceLoaders?

Pyrofab · 2020-01-07T08:17:45Z

I think we would have to load mods in several passes, using native services is begging for classloading issues.

LemmaEOF · 2020-01-07T08:18:47Z

yeah, and loading mods in several passes seems like a wide open door for issues, along with kinda destroying one of the main points of fabric loader.

asiekierka · 2020-01-07T09:37:07Z

Loading mods in several passes is not a big deal, as long as the passes are entirely data-driven and not decided by mod code - that is, they can still happen before classpath injection or any other part of loading mods.

Pyrofab · 2020-01-07T10:42:00Z

But then that removes quite a bit of flexibility for those mod providers (eg. no conditional mod loading)

modmuss50 · 2020-01-07T10:58:03Z

One issue we had when adding mc as a builtin mod, was api was trying to load it as a resource pack when it shouldnt have done.

ModResourcePackUtil.java#L43

There will need to be a way for api to know if it should load the resources, and ideally done in a way without breaking existing api versions?

i509VCB · 2020-01-07T17:52:45Z

EDIT: I am going to write up a doc explaining my concept in much more detail.

So we all seem to be asking for different specs.

If we want full capability mod providers we have to load them before the game provider starts, which would thereby start mixin.

A data driven system does not go far enough for some contexts such as plugin loading (with sponge for example). I would suggest an entrypoint which allow you to define a mod with a container, and optionally the source of the mod within class path.

This entrypoint will be loaded on a special classloader which will crash if MC is referred to and has no access to api outside of fabric loader and some basic deps.

Addition 1:

I think the instant before preLaunch is invoked we should lock the mod loading process, no more providing mods.

Addition 2:

To use this system we should require a value in the fabric.mod.json be set (not just a custom value) to use the system, as a way to require an opt in instead of people just throwing their provide_mods entrypoint in and calling it a day.

With all mods which are provided by this system, their mod container should be an instanceof a ProvidedModContainer (As a way to distinguish between provided and regular mods).

A ProvidedModBuilder should be in the api to allow provided mods to be built by code, this will return a ProvidedModContainer with capability of specifying metadata down to even mixins and entrypoints.

To keep with the contractual detail of older versions, all ModContainers, once built should be immutable. Also pyrofab's mention of this in an above comment with possibly merging mod containers and metadata with same versions, I think we should crash and burn in that case since with jar mods, we don't support multiple mods with same ID.

Pyrofab · 2020-01-07T18:49:51Z

My comment was regarding a provided mod that has the id of a real mod, but with the latter not being present. In that case, there is a decent chance the provided mod is supposed to work exactly like the missing real mod.

liach · 2021-03-19T00:29:11Z

Is this covered by the "provides" functionality?

LemmaEOF · 2021-03-19T00:32:44Z

As far as I can tell, no; provides is statically-defined and so wouldn't work for loaders that may provide arbitrary mods depending on what's in their loading directory.

LemmaEOF added 2 commits January 6, 2020 14:47

set up a simple fake-mod system

e064551

add isModLoaded support for fake mods

8c61932

i509VCB reviewed Jan 6, 2020

View reviewed changes

i509VCB mentioned this pull request Jan 7, 2020

Feature Request: Fake ModContainers #175

Open

Pyrofab mentioned this pull request Jan 13, 2020

Add a log message when a none mod is found in the mods dir, or within a mod #193

Open

LogicFan mentioned this pull request Jul 30, 2021

[Discussion] A super-early-entry-point (or loader-plugin) #486

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fake mod containers #192

Fake mod containers #192

LemmaEOF commented Jan 6, 2020

i509VCB left a comment

i509VCB Jan 6, 2020

LemmaEOF Jan 7, 2020

i509VCB Jan 7, 2020

i509VCB Jan 6, 2020

LemmaEOF Jan 7, 2020

i509VCB Jan 6, 2020

LemmaEOF Jan 7, 2020

i509VCB Jan 6, 2020

i509VCB Jan 6, 2020

LemmaEOF Jan 7, 2020

Pyrofab Jan 7, 2020 •

edited

Loading

modmuss50 commented Jan 7, 2020

i509VCB commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

asiekierka commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

modmuss50 commented Jan 7, 2020

i509VCB commented Jan 7, 2020 •

edited

Loading

Pyrofab commented Jan 7, 2020

liach commented Mar 19, 2021

LemmaEOF commented Mar 19, 2021

		@@ -0,0 +1,13 @@
		package net.fabricmc.api;

		import net.fabricmc.loader.api.ModContainer;

		@@ -59,6 +59,12 @@ static FabricLoader getInstance() {
		*/
		Collection<ModContainer> getAllMods();

Fake mod containers #192

Are you sure you want to change the base?

Fake mod containers #192

Conversation

LemmaEOF commented Jan 6, 2020

i509VCB left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Pyrofab Jan 7, 2020 • edited Loading

Choose a reason for hiding this comment

modmuss50 commented Jan 7, 2020

i509VCB commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

LemmaEOF commented Jan 7, 2020

asiekierka commented Jan 7, 2020

Pyrofab commented Jan 7, 2020

modmuss50 commented Jan 7, 2020

i509VCB commented Jan 7, 2020 • edited Loading

Pyrofab commented Jan 7, 2020

liach commented Mar 19, 2021

LemmaEOF commented Mar 19, 2021

Pyrofab Jan 7, 2020 •

edited

Loading

i509VCB commented Jan 7, 2020 •

edited

Loading